Skip to content

Fixed includes in arch/arm/src/stm32l4/hardware/.#14

Closed
wingunder wants to merge 1 commit into
apache:masterfrom
wingunder:wingunder_fixed_includes_hardware_stm32l4
Closed

Fixed includes in arch/arm/src/stm32l4/hardware/.#14
wingunder wants to merge 1 commit into
apache:masterfrom
wingunder:wingunder_fixed_includes_hardware_stm32l4

Conversation

@wingunder

Copy link
Copy Markdown
Contributor

Several header files in arch/arm/src/stm32l4/hardware/ had the
following line included:

#include "chip.h"

The problem with this was that the chip.h file is missing in this
directory (in arch/arm/src/stm32l4/hardware/). The intended chip.h
file to include was probably in arch/arm/src/stm32l4/chip.h. If any of
the headers in arch/arm/src/stm32l4/hardware/ got included from a
directory that did not contain a chip.h file, the compilation
failed.

After looking deeper into this I realized that the following files
were also including files with the wrong paths:

  • arch/arm/src/stm32l4/hardware/stm32l4_syscfg.h
  • arch/arm/src/stm32l4/hardware/stm32l4_pinmap.h

Finally, it seems as if a lot of headers, but not all, included:

#include <nuttx/config.h>

In this case, the include is only useful if a CONFIG_* define is
being referenced. These includes were removed from files that had no
CONFIG_* references and added to files that had CONFIG_* references,
but was missing the include.

The final change to be mentioned, was the moving of the GPIO defines
from arch/arm/src/stm32l4/stm32l4_gpio.h to
arch/arm/src/stm32l4/hardware/stm32l4_gpio.h. This was done to supply
arch/arm/src/stm32l4/hardware/stm32l4*_pinmap.h with a local include
for their GPIO references.

This patch changes no functionality. It simply cleans up includes and
allows inclusion of the headers in arch/arm/src/stm32l4/hardware/
from anywhere, without causing compiler errors.

Several header files in arch/arm/src/stm32l4/hardware/ had the
following line included:

   #include "chip.h"

The problem with this was that the chip.h file is missing in this
directory (in arch/arm/src/stm32l4/hardware/). The intended chip.h
file to include was probably in arch/arm/src/stm32l4/chip.h. If any of
the headers in arch/arm/src/stm32l4/hardware/ got included from a
directory that did not contain a chip.h file, the compilation
failed.

After looking deeper into this I realized that the following files
were also including files with the wrong paths:

   - arch/arm/src/stm32l4/hardware/stm32l4_syscfg.h
   - arch/arm/src/stm32l4/hardware/stm32l4_pinmap.h

Finally, it seems as if a lot of headers, but not all, included:

   #include <nuttx/config.h>

In this case, the include is only useful if a CONFIG_* define is
being referenced. These includes were removed from files that had no
CONFIG_* references and added to files that had CONFIG_* references,
but was missing the include.

The final change to be mentioned, was the moving of the GPIO defines
from arch/arm/src/stm32l4/stm32l4_gpio.h to
arch/arm/src/stm32l4/hardware/stm32l4_gpio.h. This was done to supply
arch/arm/src/stm32l4/hardware/stm32l4*_pinmap.h with a local include
for their GPIO references.

This patch changes no functionality. It simply cleans up includes and
allows inclusion of the headers in arch/arm/src/stm32l4/hardware/
from anywhere, without causing compiler errors.
@patacongo

Copy link
Copy Markdown
Contributor

I don't believe this change should be incorporated upstream. It does not handle the include paths and it takes perfectly compliant code and makes it inconsistent with everyother architecute. Consistency must be preserved.

@patacongo patacongo closed this Jan 1, 2020
@acassis

acassis commented Jan 1, 2020

Copy link
Copy Markdown
Contributor

Hi wingunder, please note that normally header files included inside stm32l4/hardware/ directory are mainly used for internal chip registers definitions and the headers at base of stm324/ contains definitions that will be outside of arch/ dir i.e. inside boards/ . Also note that all arch follow this logic, if you want to change it you need to do it for all arch, not only for stm32l4.

@wingunder

Copy link
Copy Markdown
Contributor Author

Hi @patacongo & @acassis,
Thank you for you feedback. I accept your reasoning.
Regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants